feat: forward skill catalog metadata in the reusable publish workflow by Yigtwxx · Pull Request #3414 · openclaw/clawhub

Yigtwxx · 2026-08-05T17:39:12Z

Related: #3074

What Problem This Solves

Catalog repos that publish skills through ClawHub's supported reusable workflow cannot set release or catalog metadata. .github/workflows/skill-publish.yml forwards only owner and tags, so a publisher who wants a changelog, a category, or topics on a skill has to abandon the reusable workflow and call the CLI directly.

The CLI side has been ready the whole time: clawhub skill publish declares --changelog, --categories, and --topics (packages/clawhub/src/cli.ts:494), and the package workflow is gaining the same three inputs in #3074. This closes the equivalent gap on the skill side, which I noticed while reviewing that PR.

Why This Change Was Made

The three values are optional workflow_call string inputs that travel the route the existing owner and tags inputs already use: workflow input, then an INPUT_* environment variable, then a guard that treats a blank value as absent, then a conditional append onto the Python argument list that subprocess.run executes. categories and topics are trimmed on the way through, being slug lists; changelog is forwarded verbatim, for the reason in the fourth bullet below. Parsing and validation stay in the CLI and the server, so nothing is duplicated in YAML and no permission, secret, or publish path changes.

Four deliberate decisions worth reviewer attention:

They apply to every skill the run publishes, exactly like the existing tags input, because one call can process a whole root directory. This matters more than it does for tags, and docs/cli.md now says so in its own paragraph rather than leaving it to this description: publish.ts:134 skips the unchanged short-circuit whenever catalog metadata is supplied, so a catalog-wide categories or topics value — or either clear flag, which counts as supplied — releases a new patch version of every selected skill, including ones whose files did not change. docs/cli.md:212 promises the workflow "skips unchanged skills", so that suspension needed to be stated where callers read it, next to skill_path as the way to bound the blast radius. changelog is not part of that condition and leaves the skip intact.
Clearing metadata needs its own signal. The CLI separates an omitted --categories from --categories "": hasExplicitCatalogMetadata tests options.categories !== undefined (publish.ts:84), parseCsv("") returns [], and publish.test.ts:482 pins that { categories: "", topics: "" } produces { categories: [], topics: [] } in the payload. A workflow_call string input cannot express that difference — an omitted input and an explicitly empty one both arrive as "" — so a truthiness guard alone would leave callers able to set catalog metadata through the supported workflow and never able to clear it. clear_categories and clear_topics are false-default booleans, following the dry_run/json/wait_for_publication convention already in these workflows. They are per field rather than one combined flag because the CLI applies --categories "" independently of --topics. Passing a non-empty value together with its own clear flag stops the run instead of silently preferring one. changelog gets no counterpart: publish.ts:78 already reads an omitted --changelog as "".
The step now echoes the resolved command per target, mirroring package-publish.yml:526-529. Without it this change is unverifiable: a dry run prints only the publish JSON, and SkillPublishResult carries no catalog metadata, so a forwarded flag left no trace in the logs. The quoting is log-only — the argument list is what runs, shell=True appears nowhere, and a test pins that.
changelog is forwarded verbatim. skill publish --changelog stores the text it is handed (publish.ts:78), and Markdown carries meaning in exactly the whitespace a .strip() removes: leading indentation nests a list item, and two trailing spaces are a hard line break. Trimming it here would make the supported workflow alter a caller's changelog where the direct CLI does not, so only the decision whether to forward looks past surrounding whitespace — a blank input stays the no-op it is today. categories and topics keep their .strip(); they are slug lists, not prose. This was ClawSweeper's P2 blocker on the previous head and is fixed in 78111513, with its own dispatch below.

User Impact

Skill catalog repos can set changelog text, category slugs, and topics through the supported workflow instead of replacing it with a hand-rolled CLI job. Callers that omit the three inputs get byte-identical behavior: an unset workflow_call string input is "", every guard reads a blank value as absent, and no flag is appended.

Evidence

Three dispatches, each pinning the ClawHub branch by full commit SHA: the metadata-forwarding one below, recorded at ade5dc42; the control-character one further down, recorded at ba6aa780; and the changelog-whitespace one, recorded at the current head 78111513.

ba6aa780 was the head when the first two dispatches were recorded, and two things have happened since. The branch was rebased onto faab45ba to pick up the bun audit fix (#3446, merged as 8b31a7e6); the rebase replayed the same five commits with no conflicts and changed no file in this diff, and the only drift under docs/cli.md came from main itself (#3359, in the ClawPack section, four sections away from this PR's text). Then 78111513 stopped trimming the changelog input, which is the one finding the last review left open. That commit is the only change to .github/workflows/skill-publish.yml since ba6aa780, and it is two lines plus their comment: the input parsing for categories and topics, the mutual-exclusion guard, the clear branches, and quote_for_log are untouched. So the two earlier dispatches still describe the current head for everything they assert, and the changed lines have their own current-head dispatch below.

Real behavior proof: metadata forwarding. Recorded at ade5dc4268c352c267db0a4fbbd0c2143a848988. Two commits have touched .github/workflows/skill-publish.yml since: ba6aa780, which replaces shlex.quote(part) with quote_for_log(part) in the log line and adds that helper, and 78111513, which stops trimming changelog. Neither touches the input parsing for the other inputs, the conditional appends, the mutual-exclusion guard, or the executed argument list, so the four jobs below still describe the current head's forwarding behavior; each changed area is covered by its own current-head run.

A throwaway caller repo runs the reusable workflow four times in one dispatch, every job hardcoding dry_run: true, passing no repository secrets, and pinning this branch by full commit SHA: run 31038898583, all four green. Each line below is the Resolved publish command its job logged, trimmed to the flags under test:

with-metadata

... --dry-run --tags latest --changelog 'Proof run: metadata reaches the CLI; quotes and ; stay intact.' --categories automation --topics code-review,linting --source-ref refs/heads/main

without-metadata — the control, every new input omitted

... --dry-run --tags latest --source-ref refs/heads/main

clear-both — clear_categories: true, clear_topics: true

... --dry-run --tags latest --categories '' --topics '' --source-ref refs/heads/main

set-one-clear-other — categories: "automation", clear_topics: true

... --dry-run --tags latest --categories automation --topics '' --source-ref refs/heads/main

The three shapes the CLI distinguishes — flag absent, flag with a value, flag with an empty value — each come out of a real run, and the fourth job shows they are independent per field.

The conflict guard, proven by a failing run. run 31038901710 passes categories: "automation" together with clear_categories: true and is expected to fail; the failure is the assertion. It lives in its own dispatch because continue-on-error is not accepted on a job that calls a reusable workflow, and I would rather have one red run that means something than a green one that hides it. The log:

clear_categories cannot be combined with a non-empty categories input; got categories='automation'.
Process completed with exit code 1.

Nothing is published: dry_run is hardcoded and the run stops before the first skill publish.

The changelog value deliberately carries a single quote and a semicolon; both survive as one argument, which is the property the list-based subprocess.run call guarantees.

Real behavior proof: control characters in metadata, at the current head. shlex.quote is shell quoting, not output escaping — given "a\n::notice::x" it returns 'a\n::notice::x', single quotes around a line break that is still a line break. The runner reads step stdout line by line, so a caller's newline opened a second log line that the runner parsed as a workflow command. quote_for_log (skill-publish.yml:242-249) keeps shlex.quote for the copy-pasteable common case and falls back to json.dumps when the quoted form is not printable. str.isprintable() rather than an explicit \r\n check, because it is false for every C0/C1 control character and for the Unicode line/paragraph separators, so nothing has to enumerate them and --changelog 'Adds 日本語 notes' stays on the readable path; json.dumps rather than repr, because ensure_ascii defaults to true and the fallback cannot smuggle a separator back in.

Two jobs in one dispatch, same multi-line changelog, differing only in the pinned ClawHub SHA — cc70d190 (the previously reviewed head) and ba6aa780 (the current head's workflow file, byte for byte). Both dry_run: true, no secrets: (run 31313037515, workflow source):

changelog: |-
  Line one of the changelog value.
  ::notice title=INJECTED::this line came from a changelog input

before-fix — the command log breaks in two and the runner consumes the second line:

Resolved publish command: bun ... --dry-run --tags latest --changelog 'Line one of the changelog value.
##[notice]this line came from a changelog input' --source-ref refs/heads/main

after-fix — one line, and the payload is inert text:

Resolved publish command: bun ... --dry-run --tags latest --changelog "Line one of the changelog value.\n::notice title=INJECTED::this line came from a changelog input" --source-ref refs/heads/main

The annotation API is the unambiguous half, because a line the runner accepts as a command is removed from the log rather than printed:

$ gh api repos/.../check-runs/93243729414/annotations   # before-fix
notice  INJECTED  this line came from a changelog input' --source-ref refs/heads/main
$ gh api repos/.../check-runs/93243729384/annotations   # after-fix
(empty)

One thing the run showed that is worth a maintainer's eye: the raw second line is also in both jobs' logs before the publish step runs, in the runner's own ##[group] Inputs echo of the reusable-workflow inputs and in the env: block printed above each step. Those are the runner writing its own log, not step stdout, so they are never parsed — zero annotations on after-fix is the proof of that. They cannot be suppressed from inside a workflow. A multi-line input stays visible in logs either way; what this fix removes is the part where it becomes executable.

Real behavior proof: changelog whitespace, at the current head. This is the finding the last review left open. skill publish --changelog stores the text it is handed, while this workflow read INPUT_CHANGELOG through .strip(). What that removes is not decoration: two leading spaces nest a Markdown list item, and two trailing spaces are a hard line break. A changelog published through the supported workflow therefore reached the catalog altered, and only through this workflow.

Two jobs in one dispatch, same changelog input, differing only in the pinned ClawHub SHA — f47107d1 (the head ClawSweeper reviewed) and 78111513 (the current head). Both dry_run: true, no secrets: (run 31674101726, workflow source). The input is a double-quoted scalar rather than a block scalar, so no indentation-stripping happens in YAML and both jobs receive the same bytes:

changelog: "  - indented bullet  \n  - bullet ending in a hard break  \n"

before-fix — the leading indentation and the trailing hard break are gone:

... --dry-run --tags latest --changelog "- indented bullet  \n  - bullet ending in a hard break" --source-ref refs/heads/main

after-fix — the value reaches the CLI as written:

... --dry-run --tags latest --changelog "  - indented bullet  \n  - bullet ending in a hard break  \n" --source-ref refs/heads/main

Both jobs are green: the difference is in the argument, not in the outcome, which is what makes this a fidelity bug rather than a failure anyone would have noticed. The JSON escaping in both lines is quote_for_log from the section above doing its job — the value holds newlines, so one publish still prints as one log line, and that is what makes the whitespace legible at all.

Tests.

bunx vitest run src/__tests__/skill-publish-workflow.test.ts — 7 passed at the current head, the seventh being the changelog-fidelity case added in 78111513. Two negative controls, each restoring only .github/workflows/skill-publish.yml: from f47107d1, the head ClawSweeper reviewed, 1 failed | 6 passed, the failure being the new test; from cc70d190, 3 failed | 4 passed, the two log tests and the new one. It pins the fallback itself rather than only the absence of the old join, so replacing quote_for_log with anything that leaves a control character intact fails it. The contract tests pin the whole chain per input: the parsed workflow_call input declaration, the exact INPUT_* environment expression, the Python read, and the conditional append, plus the elif clear branch, the mutual-exclusion guard, and the absence of a clear_changelog input. Reverting only the workflow file turns the clear test red, so it is not vacuous.
bunx vitest run src/__tests__/package-publish-workflow.test.ts scripts/security/package-publish-workflow.test.ts — 8 passed together with the above; the sibling workflow contracts are untouched.
bunx tsc --noEmit, bun run lint, bun run llms:check, bun run deadcode:ci, bun run check:release-workflow-action-pins — all clean.
All four Python blocks in the workflow were extracted from the parsed YAML and syntax-checked. The input-parsing and command-building slices were then exec'd verbatim across six input combinations, which produced exactly the command lines the real run later logged, including the rejection:

omitted                      -> skill publish --dry-run --tags latest
set both                     -> ... --categories automation --topics code-review,linting
clear categories only        -> ... --categories ''
clear both                   -> ... --categories '' --topics ''
set categories, clear topics -> ... --categories automation --topics ''
set + clear the same field    -> SystemExit: clear_categories cannot be combined with a non-empty categories input

bunx vitest run src/cli/commands/publish.test.ts in packages/clawhub — 20 passed, 1 failed. The failure, uploads each skill file separately before sending the publish metadata, is a pre-existing file-ordering assertion that expects SKILL.md before assets/payload.bin; it fails identically with my change stashed (1 failed | 19 passed), so this branch adds one passing test and no failure. The two clear cases in that file are the existing CLI contract this depends on, unmodified here. The new case pins the other half of the docs paragraph: a changelog-only publish of unchanged content still returns unchanged at 1.2.3, while the neighbouring test already pinned that categories/topics bump it to 1.2.4.

Gates with pre-existing failures on my machine. Both were confirmed against a clean origin/main checkout of the same tree, so neither is caused by this branch:

bun run ci:unit, re-measured at ade5dc42 against this branch's own merge base f9ea25e1 so the only difference is this PR. Branch: 15 failed | 436 passed | 1 skipped (452) files, 25 failed | 5687 passed | 9 skipped tests. Base f9ea25e1: 15 failed | 436 passed | 1 skipped (452) files, 25 failed | 5684 passed | 9 skipped tests. The sorted failing-file lists are byte-identical; the delta is the three contract tests added here. The failures are Windows-local (mkdtemp on paths containing : under scripts/, plus jsdom cases) and unrelated to workflows, docs, or tests touched here.
bun run ci:packages, same base comparison, needed because this branch now touches a file under packages/. Branch: 7 failed | 20 passed (28) files, 69 failed | 317 passed tests. Base f9ea25e1: 7 failed | 20 passed (28) files, 69 failed | 316 passed tests. Identical failing-file lists; the one-test delta is the changelog-only case added here. The failures are the same Windows-local class (curl multipart, restricted file modes, temp-directory paths).
bun run ci:static — bun audit now reports No vulnerabilities found on the rebased base, so the step that failed for most of this PR's life is gone. It stops one step later at format:check, on CLAUDE.md and .agents/skills/autoreview/CLAUDE.md; src/styles.css, the third file in the earlier reading, was fixed on main in the meantime. Neither remaining file, and no dependency, is touched by this branch.

The six red checks this PR carried were repo-wide, and their cause is now fixed on main. pr-gates was failing at bun audit on main itself — run 31332116595 at 82313c2b, 8 vulnerabilities (2 high, 5 moderate, 1 low) — and static, unit, packages, types-build, and e2e-http are mirror jobs whose only step is test "$PR_GATES_RESULT" = "success", so that one failure painted six checks red on every open PR. #3446 bumped the four advisory-hit packages and merged as 8b31a7e6; this branch is now rebased onto faab45ba, which contains it, so the checks on this head are discriminating for the first time.

Screenshots: N/A, workflow and documentation change with no UI surface.

Adjacent, now merged

package-publish.yml on main forwards the same caller-controlled changelog/categories/topics (merged in #3074) and joined them with the same " ".join(shlex.quote(part) for part in cmd) before printing that string — the quoting is load-bearing for the .sh file it also writes, but the echo carried the property this PR fixes here. #3447 applied the same helper there and merged as 8bf424cf, so quote_for_log is now the shipped shape on the package side and this PR brings the skill side to match it. The two were independent; neither needed the other to merge.

vercel · 2026-08-05T17:39:16Z

@Yigtwxx is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

clawsweeper · 2026-08-05T17:45:33Z

Codex review: needs maintainer review before merge. Reviewed August 15, 2026, 6:58 PM ET / 22:58 UTC.

ClawSweeper review

What this changes

Adds optional changelog, category, topic, and per-field metadata-clearing inputs to the reusable skill publishing workflow, with documentation and regression coverage.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open: current main and v0.23.3 still lack the skill-workflow metadata inputs, while this PR has an additive, source-aligned implementation with real reusable-workflow proof and green required checks. Likely related people: Patrick Erichsen (original skill workflow/CLI) and Sergio Peschiera (package-workflow counterpart).

Priority: P2
Reviewed head: 6d3f6a2fb2dab7a62249cb87cd922d307a931ce1

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Strong exact-head workflow proof, focused regression coverage, and a bounded compatibility-preserving patch support an above-average readiness rating.
Proof confidence	🦀 challenger crab (6/6) ✨ media proof bonus	Sufficient (linked_artifact): Linked real caller-repository dry runs demonstrate absent, populated, cleared, conflicting, control-character, and whitespace-preserving inputs; the current head retains the proven workflow code.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (linked_artifact): Linked real caller-repository dry runs demonstrate absent, populated, cleared, conflicting, control-character, and whitespace-preserving inputs; the current head retains the proven workflow code.
Evidence reviewed	7 items	Current main lacks the capability: The released skill workflow still exposes owner and tags but not catalog metadata inputs; the PR adds the missing inputs and forwarding path. CLI contract alignment: The existing CLI already distinguishes omitted categories/topics from explicit empty values and bypasses the unchanged shortcut only for explicit catalog metadata. Safe argument and logging path: The workflow appends metadata to the subprocess argument list, logs via control-character-safe quoting, and does not invoke a shell.
Findings	None	None.
Security	None	None.

How this fits together

ClawHub’s reusable GitHub Actions workflow converts catalog-repository inputs into skill-publish CLI arguments. The CLI validates the metadata and creates or updates published skill versions in the catalog.

flowchart LR
  A[Catalog repository] --> B[Reusable publish workflow]
  B --> C[Input validation]
  C --> D[Skill publish CLI]
  D --> E[Catalog metadata]
  E --> F[Published skills]

Before merge

Resolve merge risk (P1) - The repository-required autoreview could not run in this reviewer environment because TruffleHog is unavailable; a maintainer environment with that scanner can complete the procedural closeout without changing the PR.
Complete next step (P2) - No actionable patch defect remains; this PR is ready for ordinary maintainer review rather than an automated repair lane.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Change split	workflow/docs +128/-7, tests +130	Regression coverage grows slightly more than the implementation and documentation surface.
Workflow inputs	5 added inputs; 3 metadata values forwarded	The API extension is bounded and maps to existing CLI flags rather than adding a new publish path.

Merge-risk options

Maintainer options:

Decide the mitigation before merge
Land the additive input-to-CLI mapping with blank defaults, explicit per-field clear flags, and the documented catalog-wide versioning behavior.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the additive input-to-CLI mapping with blank defaults, explicit per-field clear flags, and the documented catalog-wide versioning behavior.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds an optional reusable-workflow capability rather than repairing a reported runtime failure.

Is this the best way to solve the issue?

Yes: it forwards the established CLI contract through the existing workflow argument-list path, preserving omitted-input behavior and using the merged package workflow as an adjacent pattern.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4117154ecac4.

Labels

Label justifications:

P2: This is a bounded workflow capability improvement for catalog publishers with limited blast radius.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦀 challenger crab and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): Linked real caller-repository dry runs demonstrate absent, populated, cleared, conflicting, control-character, and whitespace-preserving inputs; the current head retains the proven workflow code.
proof: sufficient: Contributor real behavior proof is sufficient. Linked real caller-repository dry runs demonstrate absent, populated, cleared, conflicting, control-character, and whitespace-preserving inputs; the current head retains the proven workflow code.

Evidence

What I checked:

Current main lacks the capability: The released skill workflow still exposes owner and tags but not catalog metadata inputs; the PR adds the missing inputs and forwarding path. (.github/workflows/skill-publish.yml:31, 6d3f6a2fb2da)
CLI contract alignment: The existing CLI already distinguishes omitted categories/topics from explicit empty values and bypasses the unchanged shortcut only for explicit catalog metadata. (packages/clawhub/src/cli/commands/publish.ts:84, 4117154ecac4)
Safe argument and logging path: The workflow appends metadata to the subprocess argument list, logs via control-character-safe quoting, and does not invoke a shell. (.github/workflows/skill-publish.yml:329, 6d3f6a2fb2da)
Exact-head proof continuity: The workflow is unchanged between the after-fix proof revision and the current PR head; the PR body supplies linked real reusable-workflow dry-run dispatches for forwarding, clearing, log safety, and Markdown whitespace. (.github/workflows/skill-publish.yml:242, 6d3f6a2fb2da)
Feature provenance: The current reusable skill workflow and its CLI metadata behavior date to the v0.23.3 commit, making Patrick Erichsen the strongest current-main routing candidate. (.github/workflows/skill-publish.yml:1, 87ca030c30f3)
Release and CI state: v0.23.3 is commit 87ca030; main has not added this workflow capability, while the supplied PR check set is fully green apart from an unrelated Vercel authorization status. (.github/workflows/skill-publish.yml:31, 4117154ecac4)

Likely related people:

Patrick Erichsen: Current-main blame attributes the reusable skill workflow and the CLI metadata handling to the v0.23.3 implementation commit. (role: original skill workflow and CLI-area contributor; confidence: high; commits: 87ca030c30f3; files: .github/workflows/skill-publish.yml, packages/clawhub/src/cli/commands/publish.ts)
Sergio Peschiera: Authored the merged package-publish metadata workflow counterpart, which is the closest established pattern for this skill workflow extension. (role: adjacent workflow-parity contributor; confidence: medium; commits: 6d935f05957b; files: .github/workflows/package-publish.yml)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (127 earlier review cycles; latest 8 shown)

reviewed 2026-08-15T13:59:11.896Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T15:05:25.924Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T16:36:36.108Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T17:02:35.077Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T18:37:04.807Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T19:42:46.450Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T20:50:41.928Z sha 6d3f6a2 :: needs maintainer review before merge. :: none
reviewed 2026-08-15T21:55:08.447Z sha 6d3f6a2 :: needs maintainer review before merge. :: none

Yigtwxx · 2026-08-05T19:27:05Z

Fixed in ade5dc42.

The finding holds, and the CLI side is more deliberate than my first version assumed:
hasExplicitCatalogMetadata tests options.categories !== undefined rather than
truthiness (publish.ts:84), parseCsv("") returns [], and publish.test.ts:482 — a
fixture named clear-topics — pins that { categories: "", topics: "" } reaches the
payload as { categories: [], topics: [] }. Clearing is a real contract with its own
regression test, and a workflow_call string input cannot reach it, because an omitted
input and an explicitly empty one both arrive as "".

clear_categories and clear_topics, false-default booleans following the
dry_run/json/wait_for_publication convention already in these workflows. Three
choices inside that:

Per field, not one combined flag. The CLI applies --categories "" independently
of --topics, so a single clear_catalog_metadata would have been coarser than the
thing it forwards.
A value plus its own clear flag stops the run rather than silently preferring one.
Picking a winner quietly is how you get a caller who thinks they cleared something.
No clear_changelog. publish.ts:78 already reads an omitted --changelog as
"", so there is nothing to disambiguate.

Real behavior, exact revision ade5dc4268c352c267db0a4fbbd0c2143a848988, four green jobs
in one dispatch, all dry_run: true with no secrets
(run 31038898583).
The logged Resolved publish command per job, trimmed:

without-metadata     ... --dry-run --tags latest --source-ref refs/heads/main
with-metadata        ... --tags latest --changelog '...' --categories automation --topics code-review,linting ...
clear-both           ... --tags latest --categories '' --topics '' ...
set-one-clear-other  ... --tags latest --categories automation --topics '' ...

All three shapes the CLI distinguishes — absent, valued, explicitly empty — come out of a
real run, and the fourth job shows the two fields move independently.

The guard has its own dispatch,
run 31038901710,
which is expected to fail — the failure is the assertion:

clear_categories cannot be combined with a non-empty categories input; got categories='automation'.
Process completed with exit code 1.

It is a separate run because continue-on-error is not accepted on a job that calls a
reusable workflow, and a red run that means something beats a green one that hides it.

Before spending the Actions runs I extracted the workflow's input-parsing and
command-building slices and exec'd them verbatim over six input combinations; they
produced exactly the lines above. The PR body now carries that table, both runs, and the
design notes.

One note for #3074, which is the same change on the package side: it forwards
categories/topics through the same truthiness guard, so the same gap is there.

Yigtwxx · 2026-08-05T19:45:26Z

Documented in 5981ee12.

The finding is right and the contrast is sharper than the note said, because
docs/cli.md:212 already promises the workflow "skips unchanged skills" — these inputs
suspend that promise a few paragraphs above where they are described. publish.ts:134
skips the already-published short-circuit whenever hasExplicitCatalogMetadata holds, so
a catalog-wide categories or topics releases a new patch version of every selected
skill.

One thing the finding did not mention, which I have documented too: the clear flags
count as supplied metadata. clear_categories: true sends --categories "", which
makes options.categories !== undefined true, so clearing a field catalog-wide
republishes everything exactly the way setting one does. That is the more surprising of
the two — "I removed a category" reads like less of an event than "I set a category".

The new paragraph sits in the notes next to skill_path, since bounding the blast radius
is the actionable half.

The republish half was already pinned by publishes explicit catalog metadata when the local skill content is unchanged (1.2.3 → 1.2.4). The changelog half was not, so it
has a test now: a changelog-only publish of unchanged content still returns unchanged
at 1.2.3 and never reaches apiRequestForm. A docs claim nobody checks is how this
paragraph goes stale.

Verification, both against this branch's own merge base f9ea25e1 so the only difference
is this PR:

ci:unit — branch 25 failed | 5687 passed, base 25 failed | 5684 passed, identical
failing-file lists.
ci:packages — branch 69 failed | 317 passed, base 69 failed | 316 passed,
identical failing-file lists. The one-test delta is the case added here. Those failures
are the machine's usual Windows-local set (curl multipart, restricted file modes,
temp-directory paths), including the uploads each skill file separately ordering
assertion in the very file I touched — it fails the same way with my change stashed.

I did not re-run the Actions proof for this commit: git diff --quiet ade5dc42 5981ee12 -- .github/workflows/skill-publish.yml passes, so the workflow those five jobs exercised is
byte-identical to the one at the current head.

pacocartones

Verify pass on ba6aa78 — no blocking findings.

Checked the claims against main (82313c2b) rather than the description:

The CLI side is as stated: skill publish declares --changelog/--categories/--topics (packages/clawhub/src/cli.ts:505-507), hasExplicitCatalogMetadata tests !== undefined (publish.ts:84), and the unchanged short-circuit skips only on explicit catalog metadata (publish.ts:134). The new CLI test pins that --changelog alone keeps the skip — good, that is the surprising corner of this design.
quote_for_log does what the evidence claims: str.isprintable() is false for any value carrying \n, \r, \t or C0/C1 controls, so those route to json.dumps, which escapes every control character and non-ASCII — one argument stays one log line regardless of input. Values that are already printable keep shlex.quote (readable logs for the common case). The execution path is untouched (subprocess.run(command, …), no shell=True), and the print happens before the run per target, so a failed publish still shows its resolved command.
The mutual-exclusion guard (clear_x + non-empty x → SystemExit) fails the step before any publish starts, and the boolean env parsing (== "true") matches how GitHub renders workflow_call booleans.
Docs match behavior: main promises "skips unchanged skills" in docs/cli.md, and the PR now states next to it that categories/topics (and the clear flags) suspend that skip catalog-wide while changelog alone does not.

Ran at ba6aa78 (bun, Windows):

bunx vitest run src/__tests__/skill-publish-workflow.test.ts → 6/6 pass.
Negative control: with main's skill-publish.yml restored, the same file fails 4/6 — exactly the new forwarding/clear/log-line tests — so they exercise the change, not pass vacuously.
CLI: the new "changelog alone keeps the skip" test passes; the file is 20/21, and the one failure ("uploads each skill file separately…") fails identically on main (pre-existing, environmental).

Non-blocking notes:

The adjacent hole you flagged is real and still on main: .github/workflows/package-publish.yml (~:572) builds shell_line with the same " ".join(shlex.quote(...)) and print()s it with caller-controlled changelog/categories/topics. There the quoted line is also written to the executed .sh, where shlex.quote is the right thing — the log print is the only injectable half. Agree it should stay out of this PR's file set; suggest a maintainer-tracked follow-up issue rather than an author offer in-thread.
Nit: changelog = os.environ["INPUT_CHANGELOG"].strip() trims leading/trailing whitespace from user prose; consistent with the neighboring inputs and harmless for markdown, but the CLI itself does not strip — a multi-paragraph changelog loses its leading blank line.
The literal-content workflow tests are brittle to any reindent of the YAML heredoc; that matches the existing tests in this file, so no change requested — flagging only so a future reformat doesn't read as a regression.

The reusable skill-publish workflow forwarded only owner and tags, so catalog repos could not set changelog, category, or topic metadata through the supported path even though `clawhub skill publish` has accepted the matching flags all along. The three values travel the existing environment-variable to argument-list route, which keeps parsing and validation in the CLI and server and leaves callers that omit the inputs on exactly their current behavior. The step now also echoes the resolved command for each target, mirroring the package workflow. Without it a forwarded flag is invisible in the run logs, because a dry run reports only the publish JSON and that payload carries no catalog metadata.

The CLI distinguishes an omitted `--categories` from `--categories ""`: the first leaves the stored slugs alone, the second clears them, and `publish.test.ts` pins that difference. A `workflow_call` string input collapses both to `""`, so the truthiness guards forwarding these values made catalog metadata settable but never clearable through the supported workflow. Two false-default booleans carry the distinction across that boundary, one per field so the CLI's per-flag independence survives. Setting a non-empty value and its clear flag together fails the run rather than silently picking one. `changelog` needs no counterpart: the CLI already reads an omitted `--changelog` as empty.

`docs/cli.md:212` promises the reusable workflow skips unchanged skills, and the new metadata inputs quietly suspend that. `publish.ts:134` skips the already-published short-circuit whenever catalog metadata is supplied, so a catalog-wide `categories` or `topics` value - or either clear flag, which counts as supplied - releases a new patch version of every selected skill. A caller editing one field can release a whole catalog without meaning to. `changelog` is not part of that condition and leaves the skip intact. The republish half was already pinned by an existing CLI test; the changelog half was not, so it gets one now.

docs/cli.md and docs/publishing.md both still said the reusable skill workflow has no categories or topics input, which contradicts the inputs this branch adds a few sections later. Both paragraphs now describe the metadata inputs, the clear flags, and the unchanged-skill skip they suspend.

shlex.quote is shell quoting, not output escaping. It wraps a value holding a line break in single quotes and leaves the break itself intact, so a changelog, categories or topics input containing a newline made the resolved command log emit a second line, which the runner parses as a workflow command. Escape the parts that are not printable so one publish stays one log line regardless of what the caller sends.

Yigtwxx · 2026-08-12T05:34:57Z

Thanks for the verify pass, @pacocartones — especially for running the negative control rather than
taking the test count at face value. Three answers, then what changed on the branch.

1. The adjacent package-publish.yml hole is closed. That was #3447, and it merged as 8bf424cf
about an hour ago, so quote_for_log is now the shipped shape on the package side too and no
follow-up issue is needed. Your reading of it matches the PR exactly: the quoted line is also written
to the executed .sh, where shlex.quote is correct, so only the print was changed there.

2. The .strip() on INPUT_CHANGELOG stays as it is, and I would rather write the reason down
than leave it as an unspoken choice. It is what the three neighbouring inputs do, and the value it
trims is leading and trailing whitespace around Markdown prose, where neither is rendered. Against
that, the workflow file at this head is byte-identical to ba6aa780 — the revision the pinned
before/after dispatch was recorded at — so any edit to it, cosmetic or not, invalidates that run as
evidence and costs a full re-proof. If a maintainer wants the CLI's exact non-stripping behavior
mirrored here, it is a one-line change and I will re-record the dispatch for it; I do not think it
earns that on its own.

3. Agreed on the literal-content tests. They are brittle to a reindent of the YAML heredoc, and
that is the existing convention in src/__tests__/skill-publish-workflow.test.ts rather than
something this PR introduces. Changing it would mean rewriting the file's other tests too.

The branch is rebased onto faab45ba. The reason is the one ClawSweeper named: the required
checks were non-discriminating, because pr-gates was failing at bun audit on main itself and
five mirror jobs only re-report its result. #3446 fixed that and merged as 8b31a7e6, so the head
now sits on top of it and the checks mean something for the first time.

What the rebase did and did not change:

Five commits replayed with no conflicts. git diff --quiet ba6aa780 HEAD is silent for all five
paths in this diff, so every pinned run in the body still describes the current head byte for byte.
The only drift under docs/cli.md came from main (feat(claws): publish exact built artifacts #3359, in the ClawPack section, four sections
away from this PR's text).
bunx vitest run src/__tests__/skill-publish-workflow.test.ts → 6 passed at the rebased head.
bun run ci:static → bun audit now reports No vulnerabilities found. It stops one step later
at format:check on CLAUDE.md and .agents/skills/autoreview/CLAUDE.md, both pre-existing and
untouched here; src/styles.css, the third file in my earlier reading, was fixed on main in the
meantime.

The body is updated for all of the above.

The reusable skill workflow read INPUT_CHANGELOG through .strip() before appending it to the publish command, while `skill publish --changelog` stores whatever text it is handed. Markdown carries meaning in exactly the whitespace that trimming removes: leading indentation nests a list item, and two trailing spaces are a hard line break. A caller's changelog reached the catalog altered, and only through this workflow. Read the value verbatim and keep the trimming only where it decides whether to forward at all, so a blank input stays the no-op it is today. `categories` and `topics` remain trimmed; they are slug lists rather than prose.

Yigtwxx · 2026-08-13T06:34:31Z

Fixed in 78111513, and I was wrong to leave it.

My last comment defended the .strip() on the grounds that it matches the three neighbouring
inputs and that touching the workflow file costs a re-proof. Both are true and neither is the
point. The neighbours are owner, tags, categories and topics — slug and identifier lists,
where surrounding whitespace is noise. changelog is Markdown prose, and skill publish --changelog stores the text it is handed (publish.ts:78), so the workflow was the only thing in
the path altering it. Two leading spaces nest a list item; two trailing spaces are a hard line
break. The finding names exactly that.

The repair is two lines and a comment:

changelog = os.environ["INPUT_CHANGELOG"]   # was .strip()
...
if changelog.strip():                       # was: if changelog
    command += ["--changelog", changelog]

The value is forwarded verbatim; only the decision whether to forward looks past whitespace, so a
blank or whitespace-only input stays the no-op it is today and callers who omit the input are
unaffected. categories and topics keep their .strip().

Real behavior proof, recorded at the new head. Two jobs in one dispatch, same changelog, both
dry_run: true with no secrets:, differing only in the pinned ClawHub SHA — f47107d1, the head
you reviewed, and 78111513 — run 31674101726,
workflow source.
The input is a double-quoted scalar, not a block scalar, so YAML performs no indentation stripping
of its own and both jobs receive the same bytes:

changelog: "  - indented bullet  \n  - bullet ending in a hard break  \n"

before-fix:

... --changelog "- indented bullet  \n  - bullet ending in a hard break" --source-ref refs/heads/main

after-fix:

... --changelog "  - indented bullet  \n  - bullet ending in a hard break  \n" --source-ref refs/heads/main

Both jobs are green, which is the shape of this bug: the difference is in the argument, not in the
outcome. The JSON escaping is quote_for_log, and it is what makes the whitespace legible in a log
at all.

On the re-proof cost I cited. It was smaller than I claimed, because it is per changed line
rather than per file. 78111513 is the only change to .github/workflows/skill-publish.yml since
ba6aa780, and it touches nothing the earlier dispatches assert — not the input parsing for
categories and topics, the mutual-exclusion guard, the clear branches, or quote_for_log — so
those runs still describe this head for their own claims, and only the changed lines needed a new
dispatch. The PR body is updated to say that rather than the byte-identical claim it carried.

Tests. bunx vitest run src/__tests__/skill-publish-workflow.test.ts — 7 passed at
78111513. The seventh is a new case that pins the verbatim read and the guard separately from the
categories/topics loop, which no longer covers changelog. Two negative controls, each
restoring only the workflow file: from f47107d1, 1 failed | 6 passed, the failure being the new
test; from cc70d190, 3 failed | 4 passed. bunx oxfmt --check is clean on the three touched
files, and all four embedded Python blocks still compile.

Also documented in docs/cli.md, next to the existing metadata notes, so a caller reading about the
inputs learns that changelog survives intact and the two slug lists are trimmed.

@clawsweeper re-review

clawsweeper · 2026-08-13T06:34:35Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

State: Complete
Detail: The durable review result and its route handoff completed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/31674655253
Updated: 2026-08-13T06:39:53.407Z

Yigtwxx · 2026-08-13T06:38:17Z

The six red checks on 78111513 are the local Convex backend failing to come up, not this diff.

One shard failed — playwright-local-auth / star-sync — and pr-gates plus its five mirror jobs (static, unit, packages, types-build, e2e-http, each of which only runs test "$PR_GATES_RESULT" = "success") re-report it, so one shard paints six checks red. Its log:

Unexpected Error: TypeError: fetch failed
✖ Failed to run function "appMeta:getDeploymentInfo": Server Error
error: Local Convex did not stop serving at http://127.0.0.1:3210.

The same failure mode is on main itself: run 31632096422 at 60b02c09, where six playwright-local-auth shards plus playwright-smoke went red with the mirror-image message —

Starting local Convex at http://127.0.0.1:3210 with isolated e2e state.
Unexpected Error: TypeError: fetch failed
Local Convex did not start cleanly on attempt 1; retrying with fresh isolated state...

— one failing to start, mine failing to stop, both after the same fetch failed against the local backend. The other seven playwright-local-auth shards passed on this head, as did playwright-smoke, including publish-new-version and old-cli-publish, which are the ones that exercise skill publishing; the previous head f47107d1 was fully green on the same shard.

This diff is a workflow YAML file, a docs paragraph, and a contract test. It cannot reach a Convex backend.

I cannot re-run a shard from a fork (gh run rerun --job refuses; my repository permissions are pull only), and the only contributor-side refresh is pushing a commit — which would move the head off 78111513 and invalidate the dispatch recorded against it an hour ago. I would rather leave the proof pinned and note the flake here. Happy to force a fresh run if a maintainer prefers that.

…talog-metadata

Yigtwxx · 2026-08-13T16:36:36Z

Refreshed against current main.

60b02c09 (fix: stream legacy skill downloads, #3451) is merged in as df1a970a, so the branch is no longer a commit behind and a fresh check set is running. That is the remaining merge-risk item.

I held the head at 78111513 until the re-review had recorded its verdict against that exact SHA; that landed at 11:31 UTC, so the dry-run proof and the review are both pinned to a commit still in this branch's history. Moving the head now costs nothing.

The merge is clean and disjoint. 60b02c09 touches convex/, server/, public/api/v1/openapi.json, docs/http-api.md, specs/spec.md and two files under src/components/; this PR touches .github/workflows/skill-publish.yml, docs/cli.md, docs/publishing.md, packages/clawhub/src/cli/commands/publish.test.ts and src/__tests__/skill-publish-workflow.test.ts. No file is in both sets, and there were no conflicts.

The six red checks on the previous head were a single playwright-local-auth / star-sync shard failing to bring the local Convex backend down, mirrored by pr-gates and its five dependants. The new run will show whether that reproduces.

…talog-metadata

Yigtwxx · 2026-08-15T08:08:17Z

Refreshed again; the red on the previous head is identified and already fixed on main.

It was not a repeat of star-sync. On df1a970a all eight playwright-local-auth shards passed and pr-gates failed at its very first step, Static checks, in 23s. The five satellite checks (static, unit, packages, types-build, e2e-http) are test "$PR_GATES_RESULT" = "success" mirrors, so they went red without running anything.

The cause is repo-wide: bun audit inside ci:static picked up an advisory that is not in the script's ignore list.

nanoid  <3.3.18
  vite > postcss > nanoid
  @react-email/ui > next > postcss > nanoid
  @vercel/analytics > next > postcss > nanoid
  @vercel/speed-insights > next > postcss > nanoid
  high: nanoid: custom generators can loop indefinitely when size is zero - GHSA-2v37-7h3g-55p8

1 vulnerabilities (1 high)
error: script "ci:static" exited with code 1

Nothing to do with this branch: the diff is one workflow file, two docs and two test files, with no dependency or lockfile change. 36b775a6 (fix: update footer attribution, #3467) bumped bun.lock from nanoid@3.3.17 to nanoid@3.3.18, which postcss@8.5.23 accepts at ^3.3.16. This refresh takes main up to 4117154e and picks that up; running the exact ci:static invocation on the merged tree now prints No vulnerabilities found and exits 0.

The merge is clean and disjoint again. The two new main commits touch bun.lock, packages/schema/, fixtures/claws/, scripts/claws-*, specs/claws.md, src/components/Footer.tsx and src/lib/nav-items.ts; git log df1a970a..4117154e restricted to the five files this PR touches returns nothing, and there were no conflicts.

Local re-run of this PR's own tests on the merged tree: src/__tests__/skill-publish-workflow.test.ts 7/7 pass, and packages/clawhub/src/cli/commands/publish.test.ts 20/21. The one failure is uploads each skill file separately before sending the publish metadata, a pre-existing test this PR does not modify; it asserts SKILL.md uploads before assets/payload.bin and gets the reverse directory order on Windows. Running main's own copy of that file in the same checkout reproduces it identically (19/20 there), so it is local file-order noise, not a branch regression, and it is green on the Linux runners.

pacocartones · 2026-08-16T13:30:18Z

Closing the loop from the review side: every finding is fixed and I verified the fixes against
the current head (6d3f6a2f).

publish.ts:84 now tests !== undefined, so an explicitly empty value and an omitted one are
distinguishable, and the clear flags reach it as a real contract. The value-plus-clear-flag
hard failure (skill-publish.yml:294) is the right call — silently picking a winner is how you
get a caller who thinks they cleared something.
quote_for_log (skill-publish.yml:242-249): str.isprintable() covering all C0/C1 and
Unicode line separators, with the ASCII-only json.dumps fallback, is narrower and safer than
enumerating \r\n. Nice catch that shlex.quote is shell quoting, not output escaping.
The republish-on-explicit-metadata behavior being pinned by tests and documented next to
skill_path is what keeps the docs paragraph from going stale.
On the red checks: your two diagnoses match what I can see — the star-sync Convex flake was
already failing on main, and the nanoid advisory was repo-wide and fixed by 36b775a6.
Nothing here was this diff. The only remaining red is the Vercel authorization gate, which is a
maintainer action.
Nothing further from my side. Thanks for the pace and the per-finding proofs.

Yigtwxx requested a review from a team as a code owner August 5, 2026 17:39

Yigtwxx mentioned this pull request Aug 10, 2026

fix: caller metadata with a line break can no longer inject workflow commands into package publish logs #3447

Merged

pacocartones reviewed Aug 11, 2026

View reviewed changes

Yigtwxx added 5 commits August 12, 2026 08:32

Yigtwxx force-pushed the feat/skill-publish-catalog-metadata branch from ba6aa78 to f47107d Compare August 12, 2026 05:33

Yigtwxx mentioned this pull request Aug 12, 2026

fix: keep catalog first-page failures retryable #3457

Open

Merge remote-tracking branch 'origin/main' into feat/skill-publish-ca…

df1a970

…talog-metadata

Merge remote-tracking branch 'origin/main' into feat/skill-publish-ca…

6d3f6a2

…talog-metadata

Uh oh!

Uh oh!

Conversation

Yigtwxx commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Adjacent, now merged

Uh oh!

vercel Bot commented Aug 5, 2026

Uh oh!

clawsweeper Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

How this fits together

Before merge

Security

Review metrics

Merge-risk options

Technical review

Labels

Evidence

Rating scale

Workflow

History

Uh oh!

Yigtwxx commented Aug 5, 2026

Uh oh!

Yigtwxx commented Aug 5, 2026

Uh oh!

pacocartones left a comment

Choose a reason for hiding this comment

Uh oh!

Yigtwxx commented Aug 12, 2026

Uh oh!

Yigtwxx commented Aug 13, 2026

Uh oh!

clawsweeper Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yigtwxx commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yigtwxx commented Aug 13, 2026

Uh oh!

Yigtwxx commented Aug 15, 2026

Uh oh!

pacocartones commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yigtwxx commented Aug 5, 2026 •

edited

Loading

clawsweeper Bot commented Aug 5, 2026 •

edited

Loading

clawsweeper Bot commented Aug 13, 2026 •

edited

Loading

Yigtwxx commented Aug 13, 2026 •

edited

Loading